-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(autoconfigure): Support both FunctionCallback and ToolCallback in ToolCallingAutoConfiguration #2233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cc9a5ec to
c945741
Compare
| List<FunctionCallback> toolCallbacks) { | ||
| var staticToolCallbackResolver = new StaticToolCallbackResolver(toolCallbacks); | ||
| ObjectProvider<List<FunctionCallback>> functionCallbacksProvider, | ||
| ObjectProvider<List<ToolCallback>> toolCallbacksProvider) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ObjectProvider<List<FunctionCallback>> will provide both FunctionCallback and ToolCallback.
I think we can keep just ObjectProvider<List<FunctionCallback>> here, and in the next release we can replace it with ObjectProvider<List<ToolCallback>>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ObjectProvider<List> will provide both FunctionCallback and ToolCallback.
I'm afraid this is not true.
Run the ToolCallingAutoConfigurationTests and put a breakpoint at ToolCallingAutoConfiguration line 59. You will find that the size of the List<FunctionCallback> allFunctionAndToolCallbacks is 2, e.g. the total number of the FunctionCallbacks defined in the test.
Then the ObjectProvider<List<ToolCallback>> toolCallbacksProvider will provide the remaining 3 ToolCallback definitions
| } | ||
|
|
||
| public StaticToolCallbackProvider(List<? extends FunctionCallback> toolCallbacks) { | ||
| Assert.notNull(toolCallbacks, "ToolCallbacks must not be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding
Assert.noNullElements(toolCallbacks, "toolCallbacks cannot contain null elements");
|
LGTM |
…n ToolCallingAutoConfiguration - Extends the ToolCallingAutoConfiguration to support both FunctionCallback and ToolCallback types. - The toolCallbackResolver bean now handles both callback types through ObjectProvider injection. - Added comprehensive tests to verify the resolution of multiple function and tool callbacks. Signed-off-by: Christian Tzolov <[email protected]>
- Update ToolCallbackProvider to return FunctionCallback[] - Migrate from List to ToolCallbackProvider in configurations - Update tests to use new provider pattern Signed-off-by: Christian Tzolov <[email protected]>
…ients Refactor AsyncMcpToolCallbackProvider and SyncMcpToolCallbackProvider to handle multiple MCP clients Add ToolCallbackProvider support to ChatClient API Deprecate direct tool callback list methods in favor of providers Fix typos in Closeable class names Update MCP documentation with new examples and usage patterns Signed-off-by: Christian Tzolov <[email protected]>
d5412a7 to
d18262e
Compare
|
|
||
| ChatClientRequestSpec tools(Object... toolObjects); | ||
|
|
||
| ChatClientRequestSpec tools(ToolCallbackProvider... toolCallbackProvider); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ChatClientRequestSpec tools(ToolCallbackProvider... toolCallbackProvider); | |
| ChatClientRequestSpec tools(ToolCallbackProvider... toolCallbackProviders); |
|
|
||
| Builder defaultTools(Object... toolObjects); | ||
|
|
||
| Builder defaultTools(ToolCallbackProvider... toolCallbackProvider); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Builder defaultTools(ToolCallbackProvider... toolCallbackProvider); | |
| Builder defaultTools(ToolCallbackProvider... toolCallbackProviders); |
| import org.springframework.ai.model.function.FunctionCallback; | ||
| import org.springframework.util.Assert; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider JavaDoc here
|
|
||
| Additionally, the registered MCP Tools with all MCP clients are provided as a list of ToolCallback instances: | ||
| Additionally, the registered MCP Tools with all MCP clients are provided as a list of ToolCallback | ||
| throurgh a ToolCallbackProvider instance: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| throurgh a ToolCallbackProvider instance: | |
| through a ToolCallbackProvider instance: |
| ---- | ||
| @Bean | ||
| public List<ToolCallback> myTools(...) { | ||
| public ToolCallbackProvier myTools(...) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public ToolCallbackProvier myTools(...) { | |
| public ToolCallbackProvider myTools(...) { |
| public ToolCallbackProvier myTools(...) { | ||
| List<ToolCallback> tools = ... | ||
| return tools; | ||
| return ToolCallbackProvier.from(tools); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return ToolCallbackProvier.from(tools); | |
| return ToolCallbackProvider.from(tools); |
Signed-off-by: Christian Tzolov <[email protected]>
|
Rebased, squashed and merged at 1fdda61 |
Uh oh!
There was an error while loading. Please reload this page.